fix(history): clear stale convertible stamps on AWS EC2 RI history rows - #1105

+-------------------------------------------------+
| We don't ship debt; we refinance it into tests. |
+-------------------------------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

Independent review (claude-cc-platform reviewer, cold brief): APPROVE 60bccb65e4ad2cc7c739614e3189336f6ddd8606

No blocking findings. The PR is one commit: migration 000133 (up and down) plus an integration test. It implements option (a) from #524.

Rows touched: AWS EC2 only (provider='aws', service IN ('ec2','compute'), case-insensitive 'convertible'). NULL rows are unaffected. Up is idempotent. Down is a no-op SELECT 1.

Consumers:

• history.ts:762-763 shows Sell for 'standard' or empty.
• handler_marketplace.go:210 gates on isKnownNonStandardOfferingClass, which is false for empty.
• populateOfferingClass (:335) re-stamps the row from AWS.

So a genuinely Convertible, still-active RI shows Sell once, and its first click returns 400 and re-stamps it. The PR discloses this.

Attacked:

• go vet -tags integration on the migrations package is clean.
• go test -tags integration -run TestMigration133 passes against Docker Postgres, including the concurrent-stamp race test.
• Mutation probes, each failing the test:
  • provider='aws' filter dropped;
  • service narrowed to 'ec2';
  • LOWER() removed;
  • down migration made to re-stamp 'convertible'.

Base drift: clean merge onto current main; the three files are new.

Nits:

• Migration ordering: main ends at 000130, and 000131/000132 and 000134-000137 are claimed by open PRs feat(exchange): record an audit trail for direct-API RI exchange executes #1092, fix(analytics): cut savings_snapshots partitions at UTC month starts #1093, fix(purchase): scope purchase suppressions to the AZ, platform and tenancy bought #1110, fix(auth): bind group account scope entries to account IDs #1116, fix(plans): migrate stored pre-#608 Azure/GCP service ids to canonical slugs #1140 and fix(db): let ri_exchange_history.account_id hold Azure and GCP identifiers #1170. golang-migrate only applies versions above the current one, so if 000133 lands before 000131/000132, those two are silently skipped on already-migrated databases. These must merge in numeric order.
• The single UPDATE is unbatched. purchase_history is small, and the READ COMMITTED analysis holds.
• The write path (execution.go:1017-1019) can still stamp the configured class on re-drives. The PR defers this to a separate issue.

Independent review (claude-fleet): APPROVE 60bccb6

The migration is correct, idempotent and tightly scoped, and every probe was caught by a test. The one real risk is merge order, which nothing in the repo or CI checks.

Findings (none blocking):

• [non-blocking, process] Merge order of 000131/000132 against 000133 is not enforced.
  • Failure scenario: if fix(history): clear stale convertible stamps on AWS EC2 RI history rows #1105 merges and deploys before feat(exchange): record an audit trail for direct-API RI exchange executes #1092 (000131) and fix(analytics): cut savings_snapshots partitions at UTC month starts #1093 (000132), any database already at 133 never applies 131 or 132. m.Up() (internal/database/postgres/migrations/migrate.go:62) only applies versions above the current one.
  • The only guard is the pre-commit duplicate-number hook (.pre-commit-config.yaml:203-205), which catches duplicates, not order. Gaps are intentional in this repo (helpers_test.go:52-57).
  • Fix: merge in numeric order, or renumber at merge time.
• [non-blocking, disclosed] 000133 down.sql:6 is a no-op. A down-then-up cycle runs up again, which clears convertible stamps that populateOfferingClass already re-verified from AWS (handler_marketplace.go:335-347). Each affected RI then costs one extra refused Sell (400) to re-stamp. No data is destroyed.
• [non-blocking, disclosed] up.sql:24-28 also clears genuinely Convertible rows written after fix(purchase): stamp the purchased EC2 offering class on history #509 (the default setting is convertible, execution.go:1017-1019). This is the accepted option (a), and the PR body states the cost.

Migration order: main ends at 000130. Open PRs claim 000131 (#1092), 000132 (#1093), 000133 (#1105, unique), 000134 (#1110), 000135 (#1116), 000136 (#1140) and 000137 (#1170), with no duplicates. A lower number must deploy before any higher one, or it is silently skipped on databases that are already migrated. Nothing enforces this: CI's migrate up (ci.yml:435) runs on a fresh database, where order never matters. This PR depends on no other open PR's data.

Attacked:

• Refuted:
  • Spelling drift: the write-path slugs are exact lowercase (execution.go:1262-1279, :1018), and the WHERE clause uses the same literals.
  • Other readers of convertible: only history.ts:762-763 and isKnownNonStandardOfferingClass (handler_marketplace.go:531) read it, and both treat empty as "unknown, allow". The exchange path (handler_ri_exchange.go:1627) never reads purchase_history.offering_class.
  • Lock scope: a single UPDATE with row locks only, with no ALTER, rewrite or ACCESS EXCLUSIVE lock. The table is small.
  • Cross-tenant scope: intended, since the defect is global. Other providers and services are untouched, which is tested.
  • Earlier-migration dependency: the column comes from 000087 (nullable TEXT), and LOWER(NULL) skips NULL rows.
• Also attacked: the WHERE clause against NULL, mixed case, standard rows, and other providers and services; the concurrent standard stamp race under READ COMMITTED; and whether down is a true inverse.
• CI coverage: the test is //go:build integration, so ci.yml:466 picks it up. It runs the real migration files through MigrateToVersion against a Postgres testcontainer.
• Mutation probes, each restored and confirmed identical with cmp:
  • P1, the service filter dropped: FAILS on ri-rds-convertible.
  • P2, the LOWER(offering_class)='convertible' guard dropped: FAILS on the standard rows and the race test.
  • P3, down re-stamps NULL rows: FAILS the rollback assertions.
  • P4, provider='aws' dropped: FAILS on azure-compute-convertible.

Verified locally, on a git archive export of HEAD with Docker and no cloud credentials:

• go build ./..., go vet and go vet -tags integration on the migrations package: clean.
• go test -count=1 -tags integration -run TestMigration133 ./internal/database/postgres/migrations/: both tests PASS.
• Against the base export, without the migration files: both FAIL.

Reviewed by an independent agent with no author context. Review only: nothing was pushed, and the GitHub review state is unchanged.

Local verification (macOS, exact head 60bccb65e4ad2cc7c739614e3189336f6ddd8606, detached worktree):

• go build ./..., go test -race -short -count=1 ./... and make lint pass.
• go vet -tags integration ./internal/database/... is clean.
• go test -race -tags integration -run TestMigration133 against Docker Postgres: 2 PASS, 0 SKIP, 0 FAIL.

Gate status: two independent reviews approve this SHA. Merge waits on @IAmJSD marking the PR ready (see #974), and on migration order: 000131 (#1092) and 000132 (#1093) must merge first, because golang-migrate skips lower versions on an already-migrated database.

Local verification at 60bccb65e4ad2cc7c739614e3189336f6ddd8606 merged onto current main (4e2ed58a; merge-tree 56b9e185; macOS):

• go build ./..., go vet -tags integration ./... and go test -race -short ./... pass (full suite).
• go test -tags integration ./internal/database/... passes on real Postgres (testcontainers), with every migration now on main applied first.
• Nothing was applied to any cloud.

Marked ready and merged by the owner's agent after 2 independent reviews and local verification (see the review and verification comments above, all at 60bccb65e4ad2cc7c739614e3189336f6ddd8606).